-
-
Notifications
You must be signed in to change notification settings - Fork 420
Fix handling of input of type FormData for the Fetch client #1008
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
29fc4dc to
6d0ec6d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 5 out of 25 changed files in this pull request and generated no comments.
Files not reviewed (20)
- templates/base/http-clients/fetch-http-client.ejs: Language not supported
- tests/spec/another-query-params/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/custom-extensions/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/defaultAsSuccess/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/defaultResponse/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/deprecated/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/dot-path-params/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/enumNamesAsValues/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/extractRequestBody/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/extractRequestParams/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/extractResponseBody/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/extractResponseError/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/jsAxios/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/moduleNameFirstTag/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/moduleNameIndex/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/on-insert-path-param/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/patch/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/responses/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/singleHttpClient/snapshots/basic.test.ts.snap: Language not supported
- tests/spec/sortTypes-false/snapshots/basic.test.ts.snap: Language not supported
|
How long will this fix stay open? The problem is actual 😤 |
|
Hi everyone, resolving this issue can be helpfull for nodejs projects. |
|
+1, I encountered the same problem. So far, I have been adding this early return manually each time after generating a new build. Therefore, it would be very helpful if this could be checked and merged. |
ad65060 to
36a852f
Compare
🦋 Changeset detectedLatest commit: 25e12e3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Signed-off-by: Sora Morimoto <[email protected]>
Several open issues are caused by the
multipart/form-dataformatter trying to provide a simple API based on an input of typeObject. However,Objectcan not express some of the typical use cases of a multipart request. On the other hand,FormDatacould.One example (mentioned in #705 and #965) is sending a requested with multiple entries belonging to the same "key". Since a
FormDataobject allows one key to be mapped to multiple values, one can simply do:This is clearly not possible with an
Object, where the key-value relationship is 1:1. #1000 tries to achieve this by automatically convertingArrayvalues to multipleFormDatakeys. However, I argue that it is not necessarily what the user wants, since they could either expect:FormDataentry, orFormDataentries.In principle, it is not so easy to infer which of the two is the one desired by the user.
Currently, however, inputs of type
FormDataare not correctly handled by themultipart/form-dataformatter for the Fetch client, which tries to enumerate the keys using theObject.keys(...)construct:swagger-typescript-api/templates/base/http-clients/fetch-http-client.ejs
Lines 107 to 108 in 325e308
Unfortunately,
FormDatareturns an empty list when used throughObject.keys()This PR proposes a way to allow the user to directly supply a
FormData, basically opting out of any smart formatting logic that might actually get in the way. In this way, the user takes responsibility for correctly managing the conversion.I argue that the breaking change is minimal since any use of
FormDatais broken under the current API. Additionally, this would align with the logic for the Axios client, which already short-circuits the formatting forFormDatainputs.swagger-typescript-api/templates/base/http-clients/axios-http-client.ejs
Lines 81 to 82 in 325e308